-
Notifications
You must be signed in to change notification settings - Fork 205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RT490: Add supported charset #1088
Conversation
@@ -421,6 +421,9 @@ class RT490Radio(chirp_common.CloneModeRadio): | |||
(0x3FC0, 0x4000) | |||
] | |||
|
|||
_valid_chars = chirp_common.CHARSET_ALPHANUMERIC + \ | |||
"`~!@#$%^&*()-=_+[]\\{}|;':\",./<>?" | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't used anywhere so this won't have any effect. I assume you want to set valid_characters
in get_features()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And yet it works, so I don't know what to tell you.
I took the example directly from mml_jc8810.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It really doesn't. I changed it to just 'a'
and I can still put in names like "FOO". With and without your patch, I can only put in uppercase characters.
The mmp_jc8810.py
actually references it exactly where I said, which is why it works there and does not work here:
https://github.com/kk7ds/chirp/blob/master/chirp/drivers/mml_jc8810.py#L470
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, crap. I clearly missed that while I was trying to not re-contaminate with the NEEDS_COMPAT_SERIAL that still existed in my original file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you squash these into a single commit (per the PR guidelines template)? The delta goes to the mailing list on every build and there's no need to tell 2000 people about both changes, just the fix :)
Thanks!
And also please give that commit a useful commit message so the recipients know what you fixed. Please also (per the guidelines) put |
Done. |
Added additional characters supported by the RT-490 display. Tested against MMLradio JC-8629.
Fixes issues 11020 and 11309.